build: switch from prettier to 11x faster oxfmt 🚀 - #8382
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe repository adopts Oxfmt and replaces Prettier formatting commands and dependencies. Documentation, configuration, source, site, fixtures, tests, and TypeScript declarations are reformatted. Several nullish-coalescing fallback expressions receive explicit grouping. Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| configPath?: string | ||
| siteId?: string | ||
| get id(): string | undefined | ||
| set id(id: string): void |
There was a problem hiding this comment.
This was invalid TypeScript, obscured by skipLibCheck: true but caught by oxfmt's parser.
| // This module is "TypeScript" but contains no actual type annotations, so | ||
| // the resulting `.d.ts` file is just useless `any`s. | ||
| declare module 'lambda-local' { | ||
| declare interface Options { |
There was a problem hiding this comment.
This was invalid TypeScript, obscured by skipLibCheck: true but caught by oxfmt's parser.
- `prettier --check`: 3413 ms median over 5 runs - `oxfmt --check`: 304 ms median over 5 runs Instead of porting over our exact prettier globs, this also starts formatting all oxfmt default globs, including json, toml, markdown, etc.
| "printWidth": 120, | ||
| "proseWrap": "always", | ||
| "trailingComma": "all", | ||
| "sortPackageJson": false, |
There was a problem hiding this comment.
We could turn this one later 😁. I didn't wanna touch too many things here.
There was a problem hiding this comment.
I'd be OK with enabling this one, too.
There was a problem hiding this comment.
I'll do it right away in a follow-up PR.
| "semi": false, | ||
| "singleQuote": true, | ||
| "printWidth": 120, | ||
| "proseWrap": "always", | ||
| "trailingComma": "all", |
There was a problem hiding this comment.
Would be nice to just use defaults, but this matches our Netlify code style
There was a problem hiding this comment.
I'd prefer oxfmt's defaults. We don't have a very consistent style across Netlify, and this PR is already changing the style of the repository.
oxfmt's default printwidth is 100, which seems wide enough for me, but we could leave it at 120 to avoid too dramatic a diff.
5f5564a to
bf51b9d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/PULL_REQUEST_TEMPLATE.md:
- Around line 15-17: Update the checklist text in the pull request template to
use the apostrophes in “you're” and “that's” instead of escaped backticks, and
hyphenate “incident-related.”
In `@tests/integration/__fixtures__/next-app/app/globals.css`:
- Around line 4-6: Update the --font-mono declaration to satisfy Stylelint’s
value-keyword-case rule by lowercasing the unquoted Menlo and Monaco family
names; preserve the existing font fallback order and values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0160b946-8078-4bc2-b9c1-9b1dbf7d57d6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.github/.kodiak.toml.github/PULL_REQUEST_TEMPLATE.md.oxfmtrc.json.prettierignore.prettierrc.jsonCLAUDE.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdbin/run.jsfunctions-templates/typescript/scheduled-function/{{name}}.mtslayout.htmlpackage.jsonrenovate.json5site/netlify.tomlsite/netlify/functions/error-reporting.tssite/scripts/util/generate-command-data.jssite/src/content.config.tssite/src/fonts/font-face.csssite/src/styles/custom.csssrc/commands/clone/clone.tssrc/commands/types.d.tssrc/lib/api.tssrc/utils/deploy/hash-fns.tssrc/utils/get-repo-data.tssrc/utils/live-tunnel.tssrc/utils/shell.tstests/integration/__fixtures__/dev-server-with-header-matching-edge-functions/netlify.tomltests/integration/__fixtures__/dev-server-with-header-matching-edge-functions/public/index.htmltests/integration/__fixtures__/hugo-site/layouts/_default/list.htmltests/integration/__fixtures__/next-app-without-config/app/globals.csstests/integration/__fixtures__/next-app-without-config/app/page.module.csstests/integration/__fixtures__/next-app/app/globals.csstests/integration/__fixtures__/next-app/app/page.module.csstests/integration/__fixtures__/plugin-changing-publish-dir/netlify.tomltests/integration/commands/dev/redirects.test.tstests/integration/commands/env/env-get.test.tstests/integration/commands/env/env-list.test.tstests/integration/commands/env/env-set.test.tstests/integration/commands/functions-invoke/functions-invoke.test.tstests/unit/commands/database/db-migration-pull.test.tstests/unit/commands/database/db-migrations-reset.test.tstests/unit/commands/database/db-status.test.tstests/unit/commands/database/util/api-errors.test.tstypes/ascii-table/index.d.tstypes/express-logging/index.d.tstypes/lambda-local/index.d.tstypes/maxstache-stream/index.d.tstypes/maxstache/index.d.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
💤 Files with no reviewable changes (2)
- .prettierrc.json
- .prettierignore
ndhoule
left a comment
ndhoule
left a comment
There was a problem hiding this comment.
Looks great, thanks for taking the time to make the improvement! My only note here is that I would prefer to see us use oxfmt's defaults. We could match Prettier's current settings to minimize the diff, but I think we might as well have one big diff. (I don't think splitting formatting changes across multiple commits has much benefit--we're still taking the torch to git history either way.)
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
commit: |
3 participants
Summary
This PR moves this repo's build tooling from prettier to oxfmt. This results in an ~11x speedup of formatting.
On my Apple M3 Pro:
prettier --check: 3413 ms median over 5 runsoxfmt --check: 304 ms median over 5 runsInstead of porting over our exact prettier globs, this also starts formatting all oxfmt default globs, including json, toml, markdown, etc. That accounts for much of the diff in this PR.